Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature reference gallery #999

Merged
merged 4 commits into from
Oct 8, 2023
Merged

Conversation

bikegeek
Copy link
Contributor

@bikegeek bikegeek commented Dec 3, 2022

Added the following notebooks to the Reference Gallery Xarray section:

  • bar.ipynb
  • line.ipynb
  • hist.ipynb
  • kde.ipynb
  • violin.ipynb

*(Based on the corresponding notebooks in the Pandas section and the User Guide)

In the bar.ipynb notebook in the Reference Gallery Pandas section:
Addressed the FutureWarning in the DataFrameGroupBy.mean() by explicitly setting numeric_only to True

(i.e. DataFrameGroupBy.mean() is replaced with DataFrameGroupBy.mean(numeric_only=True) )

@maximlt
Copy link
Member

maximlt commented Dec 13, 2022

Thanks @bikegeek !

@jbednar as you've been suggesting this contribution, I was wondering, doesn't this approach lead to too much duplication? For instance, that work could also be done for GeoPandas, and pretty much all the other supported backends, right?

But indeed the reference page is currently incomplete and somewhat misleading.

Instead I was thinking, how about using tags like on https://examples.pyviz.org/ to list the backends supported by the elements? Doing so by removing the current three and incomplete categories Pandas, Geopandas and Xarray; and by completing the existing reference pages (e.g. bar, line, hist, ...) with examples showing how to generate these plots from other data structures.

image

@jbednar
Copy link
Member

jbednar commented Dec 13, 2022

doesn't this approach lead to too much duplication?

Depends what you mean by duplication. If you look at the code for the xarray and pandas versions of the same plot type, about the only thing that's shared is .bar or .line; the sample data and plots are all different. So there isn't really any code duplication that could be eliminated as far as I can see. It's true there are multiple entries for each type of plot, but that's a choice for the website rather than for these notebooks. I.e. we could (and I would think, should) have a cross product of datalibrary x plottype just as we have for backends in HoloViews (matplotlib, bokeh, etc.), with the user selecting one data library and seeing the appropriate notebook for that library. E.g. with tabs, etc., selecting the right example for your data library. But each data library's example is independent of the other examples for other data libraries, and I don't see a way to avoid that while keeping the examples simple for a user of just one library (which most users will be).

@maximlt
Copy link
Member

maximlt commented Dec 13, 2022

Depends what you mean by duplication. If you look at the code for the xarray and pandas versions of the same plot type, about the only thing that's shared is .bar or .line;

I think that's because right now the reference pages are really poor in content. They don't provide the method signature, which you might expect from a reference page, with at least a description of the specific parameters for that method. That info would be duplicated across data structures. They also contain very few plots, the Pandas line page contains just one plot for instance. If you compare it to the equivalent plotly page, it's diametrically different. I think these reference pages should help the users getting their work done, they should be able to quickly find some recipes in there, instead of having to go through all the user guides. So in the end I believe these pages should focus more on the variations of the same plot type you can make, rather than on reading the data, hence the potential duplication across data structures.

we could (and I would think, should) have a cross product of datalibrary x plottype just as we have for backends in HoloViews (matplotlib, bokeh, etc.),

A few comments on this:

  • HoloViews, like hvPlot, has a third data structure dimension, which is totally ignored in its reference gallery. hvPlot does it the other way around, acknowleding support of multiple data structures but not showing the different plotting backends.
  • It seems to me that the current structure of HoloViews' reference gallery, with the notebook files being duplicated across the three plotting backends, has contributed to ossify it. You indeed have to apply your change to three different notebooks. I'd like to avoid getting into a similar situation.

@MarcSkovMadsen
Copy link
Collaborator

Totally agree with @maximlt that the reference notebooks are currently way to sparse. They should find inspiration in the plotly documentation. Plotting can be really difficult, finding the right now combination of parameters etc. is not always obvious. As a user you need the inspiration and reference examples.

Its on my long todo list to contribute something here.

@jbednar
Copy link
Member

jbednar commented Dec 14, 2022

the reference pages are really poor in content. They don't provide the method signature, which you might expect from a reference page, with at least a description of the specific parameters for that method. That info would be duplicated across data structures.

Sure, but that info should be auto-generated, and so again, only an issue for the website to avoid a user's experience of duplication, unrelated to a notebook like this that's about showing how to generate the specified type of plot from a particular data library's object. The actual reference material should be no more than one line in the source code for this page, saying "I want to pull in the auto-generated reference material for object X". Duplicating that one line across data libraries will not be an issue. Duplicating the other material referenced below, will be, but we will have to address that a different way unrelated to this PR.

They also contain very few plots, the Pandas line page contains just one plot for instance. If you compare it to the equivalent plotly page, it's diametrically different. I think these reference pages should help the users getting their work done, they should be able to quickly find some recipes in there, instead of having to go through all the user guides. So in the end I believe these pages should focus more on the variations of the same plot type you can make, rather than on reading the data, hence the potential duplication across data structures.

I agree it would be nice to have a full and complete guide to a given type of plot, with a lot more illustrations and examples. I'm not sure that's relevant to the review of this PR, though.

HoloViews, like hvPlot, has a third data structure dimension, which is totally ignored in its reference gallery.

Agreed. The lack of documentation for supported data structures is a serious problem, which this PR is a small step towards addressing.

It seems to me that the current structure of HoloViews' reference gallery, with the notebook files being duplicated across the three plotting backends, has contributed to ossify it. You indeed have to apply your change to three different notebooks. I'd like to avoid getting into a similar situation.

Agreed. I'd love to hear about solutions. Again, I'm not sure that's relevant to reviewing this PR. We don't even have a plan for that, let alone a commitment to work on it. Meanwhile, this PR shows an xarray user how to invoke a plot of the given type, within the current structure of the website, and should be reviewed in that context. Future work can be reviewed in future scenarios when those become real!

@maximlt
Copy link
Member

maximlt commented Dec 14, 2022

I agree it would be nice to have a full and complete guide to a given type of plot, with a lot more illustrations and examples. I'm not sure that's relevant to the review of this PR, though.

Do you expect that there will be a complete reference page for each data structure?

@maximlt maximlt added this to the 0.9.0 milestone Sep 11, 2023
@maximlt maximlt self-requested a review October 2, 2023 08:25
@maximlt
Copy link
Member

maximlt commented Oct 8, 2023

@bikegeek here's the beauty of open-source, things can take a while but they eventually happen! Thanks a lot for adding these new reference guides, I'm going to merge your PR now which is going to be included in the next release of hvPlot that is coming soon.

@maximlt maximlt merged commit 68235f1 into holoviz:main Oct 8, 2023
7 checks passed
ahuang11 pushed a commit that referenced this pull request Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants